Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: internal objects optimization (BaseAccount, Balance & Supply) #320

Merged
merged 6 commits into from
Sep 10, 2021

Conversation

iproudhon
Copy link
Contributor

  • replaced cdc.(Un)MarshalInterface() with protobuf (Un)Marshal for
    internal objects: BaseAccount, Balance & Supply
  • PubKey in BaseAccount has three seperate entries for publick keys to
    avoid MarshalInterface.
  • custom json marshaler for BaseAccount to show a single pub key for
    json compatibility

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer
  • Review Codecov Report in the comment section below once CI passes

* replaced cdc.(Un)MarshalInterface() with protobuf (Un)Marshal for
  internal objects: BaseAccount, Balance & Supply
* PubKey in BaseAccount has three seperate entries for publick keys to
  avoid MarshalInterface.
* custom json marshaler for BaseAccount to show a single pub key for
  json compatibility
@codecov
Copy link

codecov bot commented Sep 9, 2021

Codecov Report

❗ No coverage uploaded for pull request base (main@84db942). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #320   +/-   ##
=======================================
  Coverage        ?   53.11%           
=======================================
  Files           ?      642           
  Lines           ?    47570           
  Branches        ?        0           
=======================================
  Hits            ?    25267           
  Misses          ?    19416           
  Partials        ?     2887           

@egonspace
Copy link

Let's update CHANGELOG.

Woosang Son and others added 3 commits September 10, 2021 11:34
* fix: treat addresses as strings

* fix: compile errors, test failures

* fix: works-1

* feat: integrate ostracon

* fix: bump up ostracon

* fix: lint error

* fix: bump up ostracon

* fix: self review

* fix: modify changelog

* fix: disable arm building

* fix: bls build failure on arm

* fix: bls build failure on arm

* fix: bls build failure on arm
@iproudhon
Copy link
Contributor Author

Updated CHANGELOG

Copy link

@egonspace egonspace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

x/auth/types/account.go Show resolved Hide resolved
x/bank/keeper/send.go Show resolved Hide resolved
@iproudhon iproudhon merged commit 39afd48 into main Sep 10, 2021
@iproudhon iproudhon deleted the iproudhon/codec-update branch September 10, 2021 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants